Skip to content

Conversation

@woodruffw
Copy link
Member

@woodruffw woodruffw commented Dec 20, 2016

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

Just replacing usages of install_name_tool and otool with calls to ruby-macho, where possible.

There's still quite a few more formulae to convert, so this is WIP.

Remaining:

  • macvim

Not applicable:

  • cctools
  • ucommon

@woodruffw woodruffw added the in progress Stale bot should stay away label Dec 20, 2016
@ilovezfs
Copy link
Contributor

==> brew test amap --verbose
==> FAILED
Testing amap
==> Using the sandbox
/usr/bin/sandbox-exec -f /tmp/homebrew20161220-1355-gc7zfi.sb /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/bin/ruby -W0 -I /usr/local/Homebrew/Library/Homebrew -- /usr/local/Homebrew/Library/Homebrew/test.rb /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/amap.rb --verbose
Error: amap: failed
Expected ["/usr/local/opt/openssl/lib/libssl.1.0.0.dylib",
 "/usr/local/opt/openssl/lib/libcrypto.1.0.0.dylib",
 "/usr/lib/libSystem.B.dylib"] to include "/usr/local/opt/openssl/lib".
==> brew install --build-bottle --verbose libgcrypt
...
Error: No such dylib name: /usr/local/Cellar/libgcrypt/1.7.5/lib/libgcrypt.20.dylib

@woodruffw
Copy link
Member Author

woodruffw commented Dec 20, 2016

I guess I can fix the first one by mapping the dylibs to their parent directories, but I'm not sure why that was a test in the first place (it doesn't test the formula's functionality or really mean anything in terms of it working). Thoughts on removing that particular part of the test (and replacing it with something else)?

Edit: Same goes for vim and macvim - is there a reason we're testing for dylibs in their test blocks instead of feeding them a basic command sequence to see if they actually execute?

@woodruffw
Copy link
Member Author

The second failure is a bit strange, since there definitely is a /usr/local/Cellar/libgcrypt/1.7.5/lib/libgcrypt.20.dylib in .libs/random:

bash-4.4$ otool -L .libs/random 
.libs/random:
	/usr/local/Cellar/libgcrypt/1.7.5/lib/libgcrypt.20.dylib (compatibility version 22.0.0, current version 22.5.0)
	/usr/local/opt/libgpg-error/lib/libgpg-error.0.dylib (compatibility version 21.0.0, current version 21.0.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1238.0.0)

I doubt it's a bug in ruby-macho, but I guess I have to go check that now 😢

@woodruffw
Copy link
Member Author

Attached IRB confirms this:

irb(#<MachO::MachOFile:0x007ff76a1c3d88>):004:0> @filename
=> #<Pathname:/private/tmp/libgcrypt-20161220-36566-s5knjf/libgcrypt-1.7.5/tests/.libs/random>
irb(#<MachO::MachOFile:0x007ff76a1c3d88>):005:0> linked_dylibs
=> ["/usr/local/Cellar/libgcrypt/1.7.5/lib/libgcrypt.20.dylib", "/usr/local/opt/libgpg-error/lib/libgpg-error.0.dylib", "/usr/lib/libSystem.B.dylib"]

@woodruffw
Copy link
Member Author

Never mind, I'm an idiot. Ruby-macho doesn't like Pathnames...

@woodruffw woodruffw force-pushed the ruby-machoify branch 2 times, most recently from 0a8fd66 to 0802cb9 Compare December 20, 2016 23:29
@MikeMcQuaid
Copy link
Member

Nice. I like the approach.

@woodruffw
Copy link
Member Author

I've been away from my OS X box, but I'll try to fill this PR in soon.

@woodruffw woodruffw self-assigned this Jan 9, 2017
@woodruffw woodruffw force-pushed the ruby-machoify branch 2 times, most recently from fd4de85 to 230606b Compare January 10, 2017 00:52
@woodruffw
Copy link
Member Author

404ing on libsvm, sent the maintainers an email.

pypy3 is complaining about the "alpha" in the URL, but I assume that's fine since that's the way it was.

amap is complaining about assert...include?, but assert_match doesn't do what we need here.

@woodruffw
Copy link
Member Author

Opened #8706 to fix libsvm.

@ilovezfs
Copy link
Contributor

pypy3 is complaining about the "alpha" in the URL, but I assume that's fine since that's the way it was.

yep. it would be nice to explicitly white list it so this stop happening.

@ilovezfs
Copy link
Contributor

amap is complaining about assert...include?, but assert_match doesn't do what we need here.

that's a brew bug.

@woodruffw
Copy link
Member Author

yep. it would be nice to explicitly white list it so this stop happening.

Do we already have a formula whitelist for audit, or is it something we need to add?

that's a brew bug.

Yep, I figured 😄

@ilovezfs
Copy link
Contributor

There are a variety of whitelists in diagnostic.rb

@woodruffw
Copy link
Member Author

There are a variety of whitelists in diagnostic.rb

Those don't affect audit, do they? The problem causer is this in audit.rb:

if line =~ /assert [^!]+\.include?/
  problem "Use `assert_match` instead of `assert ...include?`"
end

I can either tweak that regexp to be a little more discerning, or split the test in amap into two lines to trick audit.

@ilovezfs
Copy link
Contributor

I was talking about pypy3

@woodruffw
Copy link
Member Author

Oops, misread. Sorry!

@woodruffw woodruffw force-pushed the ruby-machoify branch 2 times, most recently from 22b218e to 1b0ae89 Compare January 10, 2017 20:27
@woodruffw
Copy link
Member Author

Pinging @zmwangx since you added aec4473 to fix Vim's otool usage - any chance of a similar fix for MacVim? Just wondering if I should expend the effort to fixing this linkage test instead of tossing it out in favor of something more practical.

@zmwangx
Copy link
Contributor

zmwangx commented Jan 11, 2017

any chance of a similar fix for MacVim?

I don't know if there's chance for a fix, but even if there is it probably won't be similar. Vim doesn't have a non-interactive mode, and for MacVim, that means however you call it, the GUI is launched, and whatever info we might need is displayed in the GUI. Anyway, not really a day to day vim user, so take my opinions with a grain of salt.

@zmwangx
Copy link
Contributor

zmwangx commented Jan 11, 2017

however you call it, the GUI is launched

Oh wait, actually, mvim --version might work. It contains

Compilation: clang -c -I. -Iproto -DHAVE_CONFIG_H -DFEAT_GUI_MACVIM -Wall -Wno-unknown-pragmas -pipe  -DMACOS_X_UNIX  -F/usr/local/opt/python/Frameworks -I/usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/include/python2.7 -I/usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/include/python2.7 -fno-strict-aliasing -fno-common -dynamic -g -O2 -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1

so I guess you can tell from -I/usr/local/Cellar/python/2.7.13/ whether it is peeking into the wrong version of Python.

@ilovezfs
Copy link
Contributor

@BrewTestBot test this please

@woodruffw
Copy link
Member Author

Sierra-only failure:

haskell-stack: This formula either does not compile or function as expected on macOS
versions newer than El Capitan due to an upstream incompatibility.
Error: An unsatisfied requirement failed this build.

@zmwangx
Copy link
Contributor

zmwangx commented Jan 11, 2017

That's the MMOSR. IIRC it won't be resolved until at least GHC 8.0.2 is released: commercialhaskell/stack#2577.

@woodruffw
Copy link
Member Author

This isn't tremendously high priority, so I'm fine with waiting until 8.0.2.

@woodruffw woodruffw added the upstream issue An upstream issue report is needed label Jan 11, 2017
@ilovezfs
Copy link
Contributor

That is normal. The point of the rpath manipulations in the haskell-stack formula is to allow the El Capitan bottle to be used on Sierra. I suggest you remove the revision bump from the Haskell-stack formula and merge this PR without pulling any bottles for any of the formulae. It doesn't need to go through CI again.

@ilovezfs ilovezfs removed the upstream issue An upstream issue report is needed label Jan 12, 2017
@woodruffw
Copy link
Member Author

Alrighty. Works for me 😄

@woodruffw
Copy link
Member Author

I'm going to do the macvim otool removal in another PR, to avoid chewing through these again.

@woodruffw woodruffw merged commit a5d58ec into Homebrew:master Jan 12, 2017
@woodruffw woodruffw deleted the ruby-machoify branch January 12, 2017 02:38
@woodruffw woodruffw removed the in progress Stale bot should stay away label Jan 12, 2017
@woodruffw woodruffw changed the title [WIP] Convert formulae to prefer ruby-macho over cctools Convert formulae to prefer ruby-macho over cctools Jan 12, 2017
@ilovezfs
Copy link
Contributor

🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants